Skip to content

escape crlf and quotes in multipart form-data field headers#2478

Open
metsw24-max wants to merge 1 commit into
yhirose:masterfrom
metsw24-max:multipart-escape-field
Open

escape crlf and quotes in multipart form-data field headers#2478
metsw24-max wants to merge 1 commit into
yhirose:masterfrom
metsw24-max:multipart-escape-field

Conversation

@metsw24-max

Copy link
Copy Markdown
Contributor

Part-header injection in multipart serialisation

serialize_multipart_formdata_item_begin writes each part's Content-Disposition and Content-Type lines by concatenating item.name, item.filename and item.content_type straight into the header, with no escaping. A name or filename carrying a CR/LF therefore injects further part headers (and a body), and one carrying a " closes the quoted-string early. When an application passes a value it received from elsewhere (a forwarded upload filename being the common case) the caller cannot prevent this from the outside, so the escaping belongs in the writer.

A filename of evil\r\nContent-Type: text/x-injected\r\n\r\nSMUGGLED currently produces a part with an attacker-chosen Content-Type and body; cli.Post("/x", items) is enough to reproduce it.

The fix escapes CR, LF and " to %0D/%0A/%22 (RFC 7578 §5.1.1, matching the WHATWG form-data escaping browsers use) inside the serialiser, so every path that goes through it is covered. Well-formed values are unchanged. I have leant towards encoding rather than rejecting to keep the existing string-returning API and behaviour stable, though rejecting outright would also be defensible if you would prefer that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant